[SideNav] Unit test coverage, refactor active state management #232925

weronikaolejniczak · 2025-08-26T10:33:57Z

Summary

Resolves https://github.com/elastic/eui-private/issues/377

On this PR, I am adding unit tests that cover the whole side navigation functionality. See previous PRs:

Changes

Added test suites: both_modes.test.tsx (expanded + collapsed mode), collapsed_mode.test.tsx, expanded_mode.test.tsx.
Refactored the active state management to be fully controlled by activeItemId, no internal state.
Refactored isActive to be: isCurrent (aria-current="page") and isHighlighted (visual indication).
Added onItemClick to the navigation component interface.
Changed the primary menu limit to 12.
Improved the roving index.

Motivation

The reason I'm adding the tests afterwards is because we were iterating fast, the requirements changed frequently and the acceptance criteria were not documented anywhere.

With this testing suite, I was able to refactor the active management state to be fully controlled through activeItemId prop without introducing regression.

Coverage

File	% Stmts	% Branch	% Funcs	% Lines	Uncovered Line #s
All files	78.17	71.47	70.77	79.48
core/packages/chrome/navigation	0	0	0	0
types.ts	0	0	0	0
.../packages/chrome/navigation/src	100	100	100	100
constants.ts	100	100	100	100
...rome/navigation/src/stories	0	0	0	0
navigation.stories.tsx	0	0	0	0	23-200
...chrome/navigation/src/tests	100	100	100	100
resize_window.ts	100	100	100	100
...hrome/navigation/src/components	84.9	82.85	92.85	84.9
navigation.tsx	84.9	82.85	92.85	84.9	...44-146,319-321
...omponents/nested_secondary_menu	94.87	69.23	100	94.87
header.tsx	100	100	100	100
menu_item.tsx	90	50	100	90	60
menu_panel.tsx	100	100	100	100
primary_menu_item.tsx	100	75	100	100	31-32
use_nested_menu.ts	83.33	50	100	83.33	24
...vigation/src/components/popover	95	90.32	88.23	100
blur_popover.ts	100	100	100	100
use_keyboard_management.ts	96.15	80	100	100	37,48-54
use_persistent_popover.ts	100	100	100	100
use_popover_hover.ts	100	100	100	100
use_popover_open.ts	80	100	60	100
...n/src/components/secondary_menu	100	80	100	100
item.tsx	100	70.58	100	100	42-47,61,86-100
section.tsx	100	100	100	100
...igation/src/components/side_nav	100	94	100	100
footer.tsx	100	100	100	100
footer_item.tsx	100	92.85	100	100	55
logo.tsx	100	100	100	100
panel.tsx	100	100	100	100
primary_menu.tsx	100	100	100	100
primary_menu_item.tsx	100	92.3	100	100	82-92
...ges/chrome/navigation/src/hooks	96.49	76.74	96	98.14
use_hover_timeout.ts	100	100	100	100
use_layout_width.ts	100	83.33	100	100	27
use_menu_header_style.ts	100	50	100	100	25-32
use_navigation.ts	100	100	100	100
use_responsive_menu.ts	94.73	68.75	80	94.44	47,63
use_roving_index.ts	94.73	81.81	100	100	25,56
use_tooltip.ts	100	100	100	100
...ges/chrome/navigation/src/utils	87.35	75	93.75	92
calculate_item_visible_count.ts	100	83.33	100	100	28
focus_first_element.ts	85.71	50	100	100	19-23
focus_main_content.ts	75	50	100	75	16
get_actual_item_heights.ts	100	100	100	100
get_focusable_elements.ts	100	100	100	100
get_has_submenu.ts	100	100	100	100
get_initial_active_items.ts	83.33	83.33	80	83.33	66-71
partition_menu_items.ts	100	100	100	100
trap_focus.ts	75	62.5	100	91.66	29
...kages/shared/kbn-i18n-react/src	58.33	25	33.33	58.33
compatiblity_layer.tsx	33.33	0	0	33.33	29-32,52
inject.tsx	0	0	0	0
provider.tsx	83.33	50	100	83.33	23
...rm/packages/shared/kbn-i18n/src	7.69	0	0	8.33
loader.ts	7.69	0	0	8.33	36-164
...ckages/shared/kbn-i18n/src/core	51.16	41.66	54.54	51.16
error_handler.ts	33.33	0	0	33.33	24-26
formats.ts	100	100	100	100
i18n.ts	51.28	45.45	60	51.28	...83,193,207-221
.../shared/kbn-test/src/jest/setup	88.88	88.88	100	88.88
emotion.js	88.88	88.88	100	88.88	43

Test Suites: 3 passed, 3 total
Tests: 73 passed, 73 total
Snapshots: 3 passed, 3 total
Time: 14.984 s, estimated 16 s

Checklist

Run node scripts/jest src/core/packages/chrome/navigation/src/ --coverage.

General

Unit or functional tests were updated or added to match the most common scenarios
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
Review the backport guidelines and apply applicable backport:* labels.

Specific

All added tests pass in CI. See below 👇🏻
All added tests should pass locally. Run yarn test:jest src/core/packages/chrome/navigation to check.
There should be a high coverage of unit tests in the navigation package. Run yarn test:jest src/core/packages/chrome/navigation --coverage to check.
All side navigation acceptance criteria are covered. Verify with the Google Doc notebook, "Acceptance criteria" tab.

src/core/packages/chrome/navigation/src/__tests__/both_modes.test.tsx

elasticmachine · 2025-09-15T16:03:37Z

Pinging @elastic/eui-team (EUI)

elasticmachine · 2025-09-15T16:42:30Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: 6909aa2

Failed CI Steps

Check Types

History

💔 Build #339481 failed cd10be1

cc @weronikaolejniczak

…mode

…no-cache --fix'

Dosant · 2025-09-16T08:59:26Z

I noticed that the tests are quite noisy with the following warnings:

console.error
      Error: Not implemented: navigation (except hash changes)

console.error
      Warning: An update to SideNavPopover inside a test was not wrapped in act(...).
      
      When testing, code that causes React state updates should be wrapped into act(...):
      
      act(() => {
        /* fire events that update state */
      });
      /* assert on the output */
      
      This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act

If there is a way to get around them, would be great

weronikaolejniczak · 2025-09-16T09:00:43Z

@Dosant you are right, that is noisy. It happens on every anchor click. Let me try silencing it with some Jest mocking.

Dosant · 2025-09-16T09:07:36Z

Great job 👍 . I clicked around and everything seems to be working as expected on Kibana side without any additional changes.

The way controlled prop works now:

Kibana sets activeItemId based on current URL
User navigates to a different app (via nav or other way)
Url is updated
Kibana updates activeItemId

So we don't even need onItemClick for active state management. We will use it later for analytics

acstll

🚢

Closes #234445 ## Summary - Added unit tests for the feedback snippet component. - Followed the GIVEN, WHEN, THEN pattern for `feedback_snippet` test file which is the one carrying most of the user interaction logic (inspired by #232925). - Included snapshot tests for both the panel and the button test files. ## Testing You can run them with: `node scripts/jest --config=src/platform/packages/shared/shared-ux/feedback_snippet/jest.config.js` **Test coverage:** (Stories and confetti animation have no tests) File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------------------------------|---------|----------|---------|---------|------------------------------------------ shared-ux/feedback_snippet/src | 91.66 | 100 | 92.85 | 91.48 | feedback_button.tsx | 100 | 100 | 100 | 100 | feedback_panel.tsx | 100 | 100 | 100 | 100 | feedback_snippet.stories.tsx | 0 | 100 | 0 | 0 | 14-37 feedback_snippet.tsx | 100 | 100 | 100 | 100 | shared-ux/feedback_snippet/src/confetti | 0 | 0 | 0 | 0 | confetti.component.tsx | 0 | 0 | 0 | 0 | 14-38

elastic#232925) ## Summary Resolves elastic/eui-private#377 On this PR, I am adding unit tests that cover the whole side navigation functionality. See previous PRs: - elastic#228162 - elastic#230392 - elastic#230787 ### Changes - Added test suites: `both_modes.test.tsx` (expanded + collapsed mode), `collapsed_mode.test.tsx`, `expanded_mode.test.tsx`. - Refactored the active state management to be fully controlled by `activeItemId`, no internal state. - Refactored `isActive` to be: `isCurrent` (`aria-current="page"`) and `isHighlighted` (visual indication). - Added `onItemClick` to the navigation component interface. - Changed the primary menu limit to 12. - Improved the roving index. ### Motivation The reason I'm adding the tests afterwards is because we were iterating fast, the requirements changed frequently and the acceptance criteria were not documented anywhere. With this testing suite, I was able to refactor the active management state to be fully controlled through `activeItemId` prop without introducing regression. ### Coverage File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ------------------------------------|---------|----------|---------|---------|------------------- All files | 78.17 | 71.47 | 70.77 | 79.48 | core/packages/chrome/navigation | 0 | 0 | 0 | 0 | types.ts | 0 | 0 | 0 | 0 | .../packages/chrome/navigation/src | 100 | 100 | 100 | 100 | constants.ts | 100 | 100 | 100 | 100 | ...rome/navigation/src/__stories__ | 0 | 0 | 0 | 0 | navigation.stories.tsx | 0 | 0 | 0 | 0 | 23-200 ...chrome/navigation/src/__tests__ | 100 | 100 | 100 | 100 | resize_window.ts | 100 | 100 | 100 | 100 | ...hrome/navigation/src/components | 84.9 | 82.85 | 92.85 | 84.9 | navigation.tsx | 84.9 | 82.85 | 92.85 | 84.9 | ...44-146,319-321 ...omponents/nested_secondary_menu | 94.87 | 69.23 | 100 | 94.87 | header.tsx | 100 | 100 | 100 | 100 | menu_item.tsx | 90 | 50 | 100 | 90 | 60 menu_panel.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 75 | 100 | 100 | 31-32 use_nested_menu.ts | 83.33 | 50 | 100 | 83.33 | 24 ...vigation/src/components/popover | 95 | 90.32 | 88.23 | 100 | blur_popover.ts | 100 | 100 | 100 | 100 | use_keyboard_management.ts | 96.15 | 80 | 100 | 100 | 37,48-54 use_persistent_popover.ts | 100 | 100 | 100 | 100 | use_popover_hover.ts | 100 | 100 | 100 | 100 | use_popover_open.ts | 80 | 100 | 60 | 100 | ...n/src/components/secondary_menu | 100 | 80 | 100 | 100 | item.tsx | 100 | 70.58 | 100 | 100 | 42-47,61,86-100 section.tsx | 100 | 100 | 100 | 100 | ...igation/src/components/side_nav | 100 | 94 | 100 | 100 | footer.tsx | 100 | 100 | 100 | 100 | footer_item.tsx | 100 | 92.85 | 100 | 100 | 55 logo.tsx | 100 | 100 | 100 | 100 | panel.tsx | 100 | 100 | 100 | 100 | primary_menu.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 92.3 | 100 | 100 | 82-92 ...ges/chrome/navigation/src/hooks | 96.49 | 76.74 | 96 | 98.14 | use_hover_timeout.ts | 100 | 100 | 100 | 100 | use_layout_width.ts | 100 | 83.33 | 100 | 100 | 27 use_menu_header_style.ts | 100 | 50 | 100 | 100 | 25-32 use_navigation.ts | 100 | 100 | 100 | 100 | use_responsive_menu.ts | 94.73 | 68.75 | 80 | 94.44 | 47,63 use_roving_index.ts | 94.73 | 81.81 | 100 | 100 | 25,56 use_tooltip.ts | 100 | 100 | 100 | 100 | ...ges/chrome/navigation/src/utils | 87.35 | 75 | 93.75 | 92 | calculate_item_visible_count.ts | 100 | 83.33 | 100 | 100 | 28 focus_first_element.ts | 85.71 | 50 | 100 | 100 | 19-23 focus_main_content.ts | 75 | 50 | 100 | 75 | 16 get_actual_item_heights.ts | 100 | 100 | 100 | 100 | get_focusable_elements.ts | 100 | 100 | 100 | 100 | get_has_submenu.ts | 100 | 100 | 100 | 100 | get_initial_active_items.ts | 83.33 | 83.33 | 80 | 83.33 | 66-71 partition_menu_items.ts | 100 | 100 | 100 | 100 | trap_focus.ts | 75 | 62.5 | 100 | 91.66 | 29 ...kages/shared/kbn-i18n-react/src | 58.33 | 25 | 33.33 | 58.33 | compatiblity_layer.tsx | 33.33 | 0 | 0 | 33.33 | 29-32,52 inject.tsx | 0 | 0 | 0 | 0 | provider.tsx | 83.33 | 50 | 100 | 83.33 | 23 ...rm/packages/shared/kbn-i18n/src | 7.69 | 0 | 0 | 8.33 | loader.ts | 7.69 | 0 | 0 | 8.33 | 36-164 ...ckages/shared/kbn-i18n/src/core | 51.16 | 41.66 | 54.54 | 51.16 | error_handler.ts | 33.33 | 0 | 0 | 33.33 | 24-26 formats.ts | 100 | 100 | 100 | 100 | i18n.ts | 51.28 | 45.45 | 60 | 51.28 | ...83,193,207-221 .../shared/kbn-test/src/jest/setup | 88.88 | 88.88 | 100 | 88.88 | emotion.js | 88.88 | 88.88 | 100 | 88.88 | 43 **Test Suites:** 3 passed, 3 total **Tests:** 73 passed, 73 total **Snapshots:** 3 passed, 3 total **Time:** 14.984 s, estimated 16 s ## Checklist Run `node scripts/jest src/core/packages/chrome/navigation/src/ --coverage`. ### General - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Specific - [ ] All added tests pass in CI. See below 👇🏻 - [ ] All added tests should pass locally. Run `yarn test:jest src/core/packages/chrome/navigation` to check. - [ ] There should be a high coverage of unit tests in the `navigation` package. Run `yarn test:jest src/core/packages/chrome/navigation --coverage` to check. - [ ] All side navigation acceptance criteria are covered. Verify with the Google Doc notebook, "Acceptance criteria" tab. --------- Co-authored-by: kibanamachine <[email protected]>

…#235943) Closes elastic#234445 ## Summary - Added unit tests for the feedback snippet component. - Followed the GIVEN, WHEN, THEN pattern for `feedback_snippet` test file which is the one carrying most of the user interaction logic (inspired by elastic#232925). - Included snapshot tests for both the panel and the button test files. ## Testing You can run them with: `node scripts/jest --config=src/platform/packages/shared/shared-ux/feedback_snippet/jest.config.js` **Test coverage:** (Stories and confetti animation have no tests) File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------------------------------|---------|----------|---------|---------|------------------------------------------ shared-ux/feedback_snippet/src | 91.66 | 100 | 92.85 | 91.48 | feedback_button.tsx | 100 | 100 | 100 | 100 | feedback_panel.tsx | 100 | 100 | 100 | 100 | feedback_snippet.stories.tsx | 0 | 100 | 0 | 0 | 14-37 feedback_snippet.tsx | 100 | 100 | 100 | 100 | shared-ux/feedback_snippet/src/confetti | 0 | 0 | 0 | 0 | confetti.component.tsx | 0 | 0 | 0 | 0 | 14-38

#232925) ## Summary Resolves elastic/eui-private#377 On this PR, I am adding unit tests that cover the whole side navigation functionality. See previous PRs: - #228162 - #230392 - #230787 ### Changes - Added test suites: `both_modes.test.tsx` (expanded + collapsed mode), `collapsed_mode.test.tsx`, `expanded_mode.test.tsx`. - Refactored the active state management to be fully controlled by `activeItemId`, no internal state. - Refactored `isActive` to be: `isCurrent` (`aria-current="page"`) and `isHighlighted` (visual indication). - Added `onItemClick` to the navigation component interface. - Changed the primary menu limit to 12. - Improved the roving index. ### Motivation The reason I'm adding the tests afterwards is because we were iterating fast, the requirements changed frequently and the acceptance criteria were not documented anywhere. With this testing suite, I was able to refactor the active management state to be fully controlled through `activeItemId` prop without introducing regression. ### Coverage File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ------------------------------------|---------|----------|---------|---------|------------------- All files | 78.17 | 71.47 | 70.77 | 79.48 | core/packages/chrome/navigation | 0 | 0 | 0 | 0 | types.ts | 0 | 0 | 0 | 0 | .../packages/chrome/navigation/src | 100 | 100 | 100 | 100 | constants.ts | 100 | 100 | 100 | 100 | ...rome/navigation/src/__stories__ | 0 | 0 | 0 | 0 | navigation.stories.tsx | 0 | 0 | 0 | 0 | 23-200 ...chrome/navigation/src/__tests__ | 100 | 100 | 100 | 100 | resize_window.ts | 100 | 100 | 100 | 100 | ...hrome/navigation/src/components | 84.9 | 82.85 | 92.85 | 84.9 | navigation.tsx | 84.9 | 82.85 | 92.85 | 84.9 | ...44-146,319-321 ...omponents/nested_secondary_menu | 94.87 | 69.23 | 100 | 94.87 | header.tsx | 100 | 100 | 100 | 100 | menu_item.tsx | 90 | 50 | 100 | 90 | 60 menu_panel.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 75 | 100 | 100 | 31-32 use_nested_menu.ts | 83.33 | 50 | 100 | 83.33 | 24 ...vigation/src/components/popover | 95 | 90.32 | 88.23 | 100 | blur_popover.ts | 100 | 100 | 100 | 100 | use_keyboard_management.ts | 96.15 | 80 | 100 | 100 | 37,48-54 use_persistent_popover.ts | 100 | 100 | 100 | 100 | use_popover_hover.ts | 100 | 100 | 100 | 100 | use_popover_open.ts | 80 | 100 | 60 | 100 | ...n/src/components/secondary_menu | 100 | 80 | 100 | 100 | item.tsx | 100 | 70.58 | 100 | 100 | 42-47,61,86-100 section.tsx | 100 | 100 | 100 | 100 | ...igation/src/components/side_nav | 100 | 94 | 100 | 100 | footer.tsx | 100 | 100 | 100 | 100 | footer_item.tsx | 100 | 92.85 | 100 | 100 | 55 logo.tsx | 100 | 100 | 100 | 100 | panel.tsx | 100 | 100 | 100 | 100 | primary_menu.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 92.3 | 100 | 100 | 82-92 ...ges/chrome/navigation/src/hooks | 96.49 | 76.74 | 96 | 98.14 | use_hover_timeout.ts | 100 | 100 | 100 | 100 | use_layout_width.ts | 100 | 83.33 | 100 | 100 | 27 use_menu_header_style.ts | 100 | 50 | 100 | 100 | 25-32 use_navigation.ts | 100 | 100 | 100 | 100 | use_responsive_menu.ts | 94.73 | 68.75 | 80 | 94.44 | 47,63 use_roving_index.ts | 94.73 | 81.81 | 100 | 100 | 25,56 use_tooltip.ts | 100 | 100 | 100 | 100 | ...ges/chrome/navigation/src/utils | 87.35 | 75 | 93.75 | 92 | calculate_item_visible_count.ts | 100 | 83.33 | 100 | 100 | 28 focus_first_element.ts | 85.71 | 50 | 100 | 100 | 19-23 focus_main_content.ts | 75 | 50 | 100 | 75 | 16 get_actual_item_heights.ts | 100 | 100 | 100 | 100 | get_focusable_elements.ts | 100 | 100 | 100 | 100 | get_has_submenu.ts | 100 | 100 | 100 | 100 | get_initial_active_items.ts | 83.33 | 83.33 | 80 | 83.33 | 66-71 partition_menu_items.ts | 100 | 100 | 100 | 100 | trap_focus.ts | 75 | 62.5 | 100 | 91.66 | 29 ...kages/shared/kbn-i18n-react/src | 58.33 | 25 | 33.33 | 58.33 | compatiblity_layer.tsx | 33.33 | 0 | 0 | 33.33 | 29-32,52 inject.tsx | 0 | 0 | 0 | 0 | provider.tsx | 83.33 | 50 | 100 | 83.33 | 23 ...rm/packages/shared/kbn-i18n/src | 7.69 | 0 | 0 | 8.33 | loader.ts | 7.69 | 0 | 0 | 8.33 | 36-164 ...ckages/shared/kbn-i18n/src/core | 51.16 | 41.66 | 54.54 | 51.16 | error_handler.ts | 33.33 | 0 | 0 | 33.33 | 24-26 formats.ts | 100 | 100 | 100 | 100 | i18n.ts | 51.28 | 45.45 | 60 | 51.28 | ...83,193,207-221 .../shared/kbn-test/src/jest/setup | 88.88 | 88.88 | 100 | 88.88 | emotion.js | 88.88 | 88.88 | 100 | 88.88 | 43 **Test Suites:** 3 passed, 3 total **Tests:** 73 passed, 73 total **Snapshots:** 3 passed, 3 total **Time:** 14.984 s, estimated 16 s ## Checklist Run `node scripts/jest src/core/packages/chrome/navigation/src/ --coverage`. ### General - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Specific - [ ] All added tests pass in CI. See below 👇🏻 - [ ] All added tests should pass locally. Run `yarn test:jest src/core/packages/chrome/navigation` to check. - [ ] There should be a high coverage of unit tests in the `navigation` package. Run `yarn test:jest src/core/packages/chrome/navigation --coverage` to check. - [ ] All side navigation acceptance criteria are covered. Verify with the Google Doc notebook, "Acceptance criteria" tab. --------- Co-authored-by: kibanamachine <[email protected]>

Closes #234445 ## Summary - Added unit tests for the feedback snippet component. - Followed the GIVEN, WHEN, THEN pattern for `feedback_snippet` test file which is the one carrying most of the user interaction logic (inspired by #232925). - Included snapshot tests for both the panel and the button test files. ## Testing You can run them with: `node scripts/jest --config=src/platform/packages/shared/shared-ux/feedback_snippet/jest.config.js` **Test coverage:** (Stories and confetti animation have no tests) File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------------------------------|---------|----------|---------|---------|------------------------------------------ shared-ux/feedback_snippet/src | 91.66 | 100 | 92.85 | 91.48 | feedback_button.tsx | 100 | 100 | 100 | 100 | feedback_panel.tsx | 100 | 100 | 100 | 100 | feedback_snippet.stories.tsx | 0 | 100 | 0 | 0 | 14-37 feedback_snippet.tsx | 100 | 100 | 100 | 100 | shared-ux/feedback_snippet/src/confetti | 0 | 0 | 0 | 0 | confetti.component.tsx | 0 | 0 | 0 | 0 | 14-38

elastic#232925) ## Summary Resolves elastic/eui-private#377 On this PR, I am adding unit tests that cover the whole side navigation functionality. See previous PRs: - elastic#228162 - elastic#230392 - elastic#230787 ### Changes - Added test suites: `both_modes.test.tsx` (expanded + collapsed mode), `collapsed_mode.test.tsx`, `expanded_mode.test.tsx`. - Refactored the active state management to be fully controlled by `activeItemId`, no internal state. - Refactored `isActive` to be: `isCurrent` (`aria-current="page"`) and `isHighlighted` (visual indication). - Added `onItemClick` to the navigation component interface. - Changed the primary menu limit to 12. - Improved the roving index. ### Motivation The reason I'm adding the tests afterwards is because we were iterating fast, the requirements changed frequently and the acceptance criteria were not documented anywhere. With this testing suite, I was able to refactor the active management state to be fully controlled through `activeItemId` prop without introducing regression. ### Coverage File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ------------------------------------|---------|----------|---------|---------|------------------- All files | 78.17 | 71.47 | 70.77 | 79.48 | core/packages/chrome/navigation | 0 | 0 | 0 | 0 | types.ts | 0 | 0 | 0 | 0 | .../packages/chrome/navigation/src | 100 | 100 | 100 | 100 | constants.ts | 100 | 100 | 100 | 100 | ...rome/navigation/src/__stories__ | 0 | 0 | 0 | 0 | navigation.stories.tsx | 0 | 0 | 0 | 0 | 23-200 ...chrome/navigation/src/__tests__ | 100 | 100 | 100 | 100 | resize_window.ts | 100 | 100 | 100 | 100 | ...hrome/navigation/src/components | 84.9 | 82.85 | 92.85 | 84.9 | navigation.tsx | 84.9 | 82.85 | 92.85 | 84.9 | ...44-146,319-321 ...omponents/nested_secondary_menu | 94.87 | 69.23 | 100 | 94.87 | header.tsx | 100 | 100 | 100 | 100 | menu_item.tsx | 90 | 50 | 100 | 90 | 60 menu_panel.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 75 | 100 | 100 | 31-32 use_nested_menu.ts | 83.33 | 50 | 100 | 83.33 | 24 ...vigation/src/components/popover | 95 | 90.32 | 88.23 | 100 | blur_popover.ts | 100 | 100 | 100 | 100 | use_keyboard_management.ts | 96.15 | 80 | 100 | 100 | 37,48-54 use_persistent_popover.ts | 100 | 100 | 100 | 100 | use_popover_hover.ts | 100 | 100 | 100 | 100 | use_popover_open.ts | 80 | 100 | 60 | 100 | ...n/src/components/secondary_menu | 100 | 80 | 100 | 100 | item.tsx | 100 | 70.58 | 100 | 100 | 42-47,61,86-100 section.tsx | 100 | 100 | 100 | 100 | ...igation/src/components/side_nav | 100 | 94 | 100 | 100 | footer.tsx | 100 | 100 | 100 | 100 | footer_item.tsx | 100 | 92.85 | 100 | 100 | 55 logo.tsx | 100 | 100 | 100 | 100 | panel.tsx | 100 | 100 | 100 | 100 | primary_menu.tsx | 100 | 100 | 100 | 100 | primary_menu_item.tsx | 100 | 92.3 | 100 | 100 | 82-92 ...ges/chrome/navigation/src/hooks | 96.49 | 76.74 | 96 | 98.14 | use_hover_timeout.ts | 100 | 100 | 100 | 100 | use_layout_width.ts | 100 | 83.33 | 100 | 100 | 27 use_menu_header_style.ts | 100 | 50 | 100 | 100 | 25-32 use_navigation.ts | 100 | 100 | 100 | 100 | use_responsive_menu.ts | 94.73 | 68.75 | 80 | 94.44 | 47,63 use_roving_index.ts | 94.73 | 81.81 | 100 | 100 | 25,56 use_tooltip.ts | 100 | 100 | 100 | 100 | ...ges/chrome/navigation/src/utils | 87.35 | 75 | 93.75 | 92 | calculate_item_visible_count.ts | 100 | 83.33 | 100 | 100 | 28 focus_first_element.ts | 85.71 | 50 | 100 | 100 | 19-23 focus_main_content.ts | 75 | 50 | 100 | 75 | 16 get_actual_item_heights.ts | 100 | 100 | 100 | 100 | get_focusable_elements.ts | 100 | 100 | 100 | 100 | get_has_submenu.ts | 100 | 100 | 100 | 100 | get_initial_active_items.ts | 83.33 | 83.33 | 80 | 83.33 | 66-71 partition_menu_items.ts | 100 | 100 | 100 | 100 | trap_focus.ts | 75 | 62.5 | 100 | 91.66 | 29 ...kages/shared/kbn-i18n-react/src | 58.33 | 25 | 33.33 | 58.33 | compatiblity_layer.tsx | 33.33 | 0 | 0 | 33.33 | 29-32,52 inject.tsx | 0 | 0 | 0 | 0 | provider.tsx | 83.33 | 50 | 100 | 83.33 | 23 ...rm/packages/shared/kbn-i18n/src | 7.69 | 0 | 0 | 8.33 | loader.ts | 7.69 | 0 | 0 | 8.33 | 36-164 ...ckages/shared/kbn-i18n/src/core | 51.16 | 41.66 | 54.54 | 51.16 | error_handler.ts | 33.33 | 0 | 0 | 33.33 | 24-26 formats.ts | 100 | 100 | 100 | 100 | i18n.ts | 51.28 | 45.45 | 60 | 51.28 | ...83,193,207-221 .../shared/kbn-test/src/jest/setup | 88.88 | 88.88 | 100 | 88.88 | emotion.js | 88.88 | 88.88 | 100 | 88.88 | 43 **Test Suites:** 3 passed, 3 total **Tests:** 73 passed, 73 total **Snapshots:** 3 passed, 3 total **Time:** 14.984 s, estimated 16 s ## Checklist Run `node scripts/jest src/core/packages/chrome/navigation/src/ --coverage`. ### General - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Specific - [ ] All added tests pass in CI. See below 👇🏻 - [ ] All added tests should pass locally. Run `yarn test:jest src/core/packages/chrome/navigation` to check. - [ ] There should be a high coverage of unit tests in the `navigation` package. Run `yarn test:jest src/core/packages/chrome/navigation --coverage` to check. - [ ] All side navigation acceptance criteria are covered. Verify with the Google Doc notebook, "Acceptance criteria" tab. --------- Co-authored-by: kibanamachine <[email protected]>

…#235943) Closes elastic#234445 ## Summary - Added unit tests for the feedback snippet component. - Followed the GIVEN, WHEN, THEN pattern for `feedback_snippet` test file which is the one carrying most of the user interaction logic (inspired by elastic#232925). - Included snapshot tests for both the panel and the button test files. ## Testing You can run them with: `node scripts/jest --config=src/platform/packages/shared/shared-ux/feedback_snippet/jest.config.js` **Test coverage:** (Stories and confetti animation have no tests) File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------------------------------|---------|----------|---------|---------|------------------------------------------ shared-ux/feedback_snippet/src | 91.66 | 100 | 92.85 | 91.48 | feedback_button.tsx | 100 | 100 | 100 | 100 | feedback_panel.tsx | 100 | 100 | 100 | 100 | feedback_snippet.stories.tsx | 0 | 100 | 0 | 0 | 14-37 feedback_snippet.tsx | 100 | 100 | 100 | 100 | shared-ux/feedback_snippet/src/confetti | 0 | 0 | 0 | 0 | confetti.component.tsx | 0 | 0 | 0 | 0 | 14-38

weronikaolejniczak changed the title ~~Chore/navigation tests~~ [Side navigation] Unit tests and coverage Aug 26, 2025

weronikaolejniczak self-assigned this Aug 26, 2025

weronikaolejniczak added backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes EUI labels Aug 26, 2025

weronikaolejniczak mentioned this pull request Sep 12, 2025

[Shared UX] Implement side nav v2 feedback snippet #234187

Merged

weronikaolejniczak force-pushed the chore/navigation-tests branch from aaf4cfa to 246badc Compare September 15, 2025 15:53

weronikaolejniczak requested a review from a team September 15, 2025 15:53

weronikaolejniczak force-pushed the chore/navigation-tests branch 2 times, most recently from fc600d4 to 1d8186a Compare September 15, 2025 15:58

weronikaolejniczak commented Sep 15, 2025

View reviewed changes

src/core/packages/chrome/navigation/src/__tests__/both_modes.test.tsx Outdated Show resolved Hide resolved

weronikaolejniczak marked this pull request as ready for review September 15, 2025 16:03

weronikaolejniczak requested a review from a team as a code owner September 15, 2025 16:03

weronikaolejniczak changed the title ~~[Side navigation] Unit tests and coverage~~ [Side navigation] Unit test coverage, refactor active state management Sep 15, 2025

weronikaolejniczak requested a review from Dosant September 15, 2025 16:09

weronikaolejniczak and others added 12 commits September 15, 2025 20:33

chore(navigation): test suite structure for both modes

f555015

chore(navigation): test suite structure for expanded mode

451ddda

chore(navigation): test suite structure for collapsed mode

7fb0ea3

fix(navigation): improve roving index

a820199

fix(navigation): fix tooltip and popover showing at once in expanded …

0e477ff

…mode

chore(navigation): add jest-dom types to TS config

5a230f3

fix(navigation): fix solution logo not displaying in Storybook

5b18312

feat(navigation): change primary menu item limit to 12

16e2053

feat: refactor active state management and test

d4879a4

chore(navigation): remove todo comment from test suite

76999f5

[CI] Auto-commit changed files from 'node scripts/eslint_all_files --…

55c5a55

…no-cache --fix'

fix(navigation): remove nullish coalescing for label prop

52c059e

weronikaolejniczak force-pushed the chore/navigation-tests branch from 16ef5ff to 52c059e Compare September 15, 2025 18:33

chore(navigation): remove a test case

9456e91

Dosant approved these changes Sep 16, 2025

View reviewed changes

acstll approved these changes Sep 16, 2025

View reviewed changes

weronikaolejniczak merged commit 3a37e08 into elastic:main Sep 16, 2025
13 checks passed

kibanamachine added the v9.2.0 label Sep 16, 2025

angeles-mb mentioned this pull request Sep 22, 2025

[SharedUX] Add unit tests for the feedback snippet component #235943

Merged

weronikaolejniczak changed the title ~~[Side navigation] Unit test coverage, refactor active state management~~ [SideNav] Unit test coverage, refactor active state management Sep 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SideNav] Unit test coverage, refactor active state management #232925

[SideNav] Unit test coverage, refactor active state management #232925

Uh oh!

weronikaolejniczak commented Aug 26, 2025 •

edited

Loading

Uh oh!

Uh oh!

elasticmachine commented Sep 15, 2025

Uh oh!

elasticmachine commented Sep 15, 2025

Uh oh!

Dosant commented Sep 16, 2025

Uh oh!

weronikaolejniczak commented Sep 16, 2025

Uh oh!

Dosant commented Sep 16, 2025

Uh oh!

acstll left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[SideNav] Unit test coverage, refactor active state management #232925

[SideNav] Unit test coverage, refactor active state management #232925

Uh oh!

Conversation

weronikaolejniczak commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Motivation

Coverage

Checklist

General

Specific

Uh oh!

Uh oh!

elasticmachine commented Sep 15, 2025

Uh oh!

elasticmachine commented Sep 15, 2025

⏳ Build in-progress, with failures

Failed CI Steps

History

Uh oh!

Dosant commented Sep 16, 2025

Uh oh!

weronikaolejniczak commented Sep 16, 2025

Uh oh!

Dosant commented Sep 16, 2025

Uh oh!

acstll left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

weronikaolejniczak commented Aug 26, 2025 •

edited

Loading